Break builder pattern to expose all setters via bindings by tnull · Pull Request #88 · lightningdevkit/ldk-node

tnull · 2023-05-09T13:23:07Z

Closes #65

Unfortunately there is no good way of exposing the builder pattern via Uniffi bindings currently. We therefore resort to internal mutability and using setters on the Builder object.

tnull · 2023-05-12T10:02:28Z

Rebased on main.

tnull · 2023-05-18T08:57:37Z

Rebased on main after #70 has landed. We now also expose the RGS setter methods via bindings.

Moreover, since we already have EntropySourceConfig and GossipSourceConfig, I now moved the Esplora server URL to a corresponding enum ChainDataSourceConfig, which will us to add further variants (e.g., Electrum) in the future without breaking the API.

Additionally, we document the set defaults and switch our default config to Network::Bitcoin while we're here.

tnull · 2023-05-23T17:26:16Z

Rebased after #85 landed.

tnull · 2023-05-25T10:15:53Z

Rebased on main after #101 landed.

src/lib.rs

bindings/kotlin/ldk-node-jvm/lib/src/test/kotlin/org/lightningdevkit/ldknode/LibraryTest.kt

src/lib.rs

tnull · 2023-05-26T08:36:42Z

I now included another commit that moves all UniFFI-specific types and trait impls to a dedicated module, which is now feature-gated behind the uniffi feature. There are three main motivations to do this:

decluttering the codebase, in particular as types.rs started to get unwieldy
UniFFI pulls in additional dependencies which Rust users don't have to put up with if they don't use it anyways and
It may allow us to have a saner MSRV for the Rust part in the future, as UniFFI currently requires something like 1.62.0.

However, I'll look into actually lowering the MSRV separately, as it may require downgrading UniFFI, and IIRC there was good reason why I had upgraded to 0.23.0 in the first place...

Unfortunately there is no good way of exposing the builder pattern via Uniffi bindings currenlty. We therefore resort to internal mutability and using setters on the `Builder` object.

jkczyz

Looks good minus a couple easy comments.

src/lib.rs

After we already have `EntropySourceConfig` and `GossipSourceConfig`, we here move the esplora server URL to a corresponding `enum ChainDataSourceConfig`, which will us to add further variants (e.g., `Electrum`) in the future without breaking the API. Additionally, we document the set default values.

As we're getting ready for release

While we need quite a bit of dependencies and custom impls for UniFFI support, there is no reason to pull them in for Rust-only builds. Here we move the UniFFI-specific types and trait impls to a dedicated module and hide it behind a new `uniffi` feature.

tnull · 2023-05-26T20:43:04Z

Updated the commits to address feedback and included:

diff --git a/src/lib.rs b/src/lib.rs
index b9b75ca..3a0526f 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -375,22 +375,18 @@ impl Builder {

                // Initialize the on-chain wallet and chain access
-               let seed_bytes = if let Some(entropy_source_config) =
-                       &*self.entropy_source_config.read().unwrap()
-               {
-                       // Use the configured entropy source, if the user set one.
-                       match entropy_source_config {
-                               EntropySourceConfig::SeedBytes(bytes) => bytes.clone(),
-                               EntropySourceConfig::SeedFile(seed_path) => {
-                                       io::utils::read_or_generate_seed_file(seed_path)
-                               }
-                               EntropySourceConfig::Bip39Mnemonic { mnemonic, passphrase } => match passphrase {
-                                       Some(passphrase) => mnemonic.to_seed(passphrase),
-                                       None => mnemonic.to_seed(""),
-                               },
+               let seed_bytes = match &*self.entropy_source_config.read().unwrap() {
+                       Some(EntropySourceConfig::SeedBytes(bytes)) => bytes.clone(),
+                       Some(EntropySourceConfig::SeedFile(seed_path)) => {
+                               io::utils::read_or_generate_seed_file(seed_path)
+                       }
+                       Some(EntropySourceConfig::Bip39Mnemonic { mnemonic, passphrase }) => match passphrase {
+                               Some(passphrase) => mnemonic.to_seed(passphrase),
+                               None => mnemonic.to_seed(""),
+                       },
+                       None => {
+                               // Default to read or generate from the default location generate a seed file.
+                               let seed_path = format!("{}/keys_seed", config.storage_dir_path);
+                               io::utils::read_or_generate_seed_file(&seed_path)
                        }
-               } else {
-                       // Default to read or generate from the default location generate a seed file.
-                       let seed_path = format!("{}/keys_seed", config.storage_dir_path);
-                       io::utils::read_or_generate_seed_file(&seed_path)
                };

@@ -417,27 +413,22 @@ impl Builder {
                .expect("Failed to set up on-chain wallet");

-               let (blockchain, tx_sync) = if let Some(chain_data_source_config) =
-                       &*self.chain_data_source_config.read().unwrap()
-               {
-                       match chain_data_source_config {
-                               ChainDataSourceConfig::Esplora(server_url) => {
-                                       let tx_sync =
-                                               Arc::new(EsploraSyncClient::new(server_url.clone(), Arc::clone(&logger)));
-                                       let blockchain = EsploraBlockchain::from_client(
-                                               tx_sync.client().clone(),
-                                               BDK_CLIENT_STOP_GAP,
-                                       )
-                                       .with_concurrency(BDK_CLIENT_CONCURRENCY);
-                                       (blockchain, tx_sync)
-                               }
+               let (blockchain, tx_sync) = match &*self.chain_data_source_config.read().unwrap() {
+                       Some(ChainDataSourceConfig::Esplora(server_url)) => {
+                               let tx_sync =
+                                       Arc::new(EsploraSyncClient::new(server_url.clone(), Arc::clone(&logger)));
+                               let blockchain =
+                                       EsploraBlockchain::from_client(tx_sync.client().clone(), BDK_CLIENT_STOP_GAP)
+                                               .with_concurrency(BDK_CLIENT_CONCURRENCY);
+                               (blockchain, tx_sync)
+                       }
+                       None => {
+                               // Default to Esplora client.
+                               let server_url = DEFAULT_ESPLORA_SERVER_URL.to_string();
+                               let tx_sync = Arc::new(EsploraSyncClient::new(server_url, Arc::clone(&logger)));
+                               let blockchain =
+                                       EsploraBlockchain::from_client(tx_sync.client().clone(), BDK_CLIENT_STOP_GAP)
+                                               .with_concurrency(BDK_CLIENT_CONCURRENCY);
+                               (blockchain, tx_sync)
                        }
-               } else {
-                       // Default to Esplora client with blockstream endpoint.
-                       let server_url = DEFAULT_ESPLORA_SERVER_URL.to_string();
-                       let tx_sync = Arc::new(EsploraSyncClient::new(server_url, Arc::clone(&logger)));
-                       let blockchain =
-                               EsploraBlockchain::from_client(tx_sync.client().clone(), BDK_CLIENT_STOP_GAP)
-                                       .with_concurrency(BDK_CLIENT_CONCURRENCY);
-                       (blockchain, tx_sync)
                };

Add HMAC-based authentication for RPC/CLI

tnull changed the title ~~2023 05 expose builder in bindings~~ Break builder pattern to expose all setters via bindings May 9, 2023

tnull force-pushed the 2023-05-expose-builder-in-bindings branch from 96a70ae to e0cc6f8 Compare May 9, 2023 13:35

tnull added this to the 0.1 milestone May 11, 2023

tnull force-pushed the 2023-05-expose-builder-in-bindings branch from acfcd7c to 0f1e7ed Compare May 12, 2023 10:02

tnull force-pushed the 2023-05-expose-builder-in-bindings branch 2 times, most recently from 0dbd449 to 070314f Compare May 18, 2023 08:57

tnull force-pushed the 2023-05-expose-builder-in-bindings branch from 070314f to 3b3a775 Compare May 18, 2023 09:08

tnull mentioned this pull request May 18, 2023

LDK Node v0.1 Tracking Issue #45

Closed

47 tasks

tnull force-pushed the 2023-05-expose-builder-in-bindings branch from 3b3a775 to a9da44a Compare May 18, 2023 09:27

tnull added the Seeking Code Review label May 19, 2023

This was referenced May 21, 2023

Make Builder methods return errors #103

Closed

Switch to use NetAddress for peer addresses #85

Closed

tnull force-pushed the 2023-05-expose-builder-in-bindings branch from a9da44a to 49baa90 Compare May 23, 2023 17:26

tnull force-pushed the 2023-05-expose-builder-in-bindings branch 3 times, most recently from cdefbe6 to b5db6d0 Compare May 23, 2023 17:53

tnull mentioned this pull request May 25, 2023

Add SqliteStore backend #100

Merged

tnull force-pushed the 2023-05-expose-builder-in-bindings branch from b5db6d0 to 7a56598 Compare May 25, 2023 10:15

tnull force-pushed the 2023-05-expose-builder-in-bindings branch from 7a56598 to bb02755 Compare May 25, 2023 12:07

tnull mentioned this pull request May 25, 2023

Expose BIP39 Mnemonic generation in bindings #113

Merged

jkczyz reviewed May 26, 2023

View reviewed changes

src/lib.rs Show resolved Hide resolved

bindings/kotlin/ldk-node-jvm/lib/src/test/kotlin/org/lightningdevkit/ldknode/LibraryTest.kt Show resolved Hide resolved

src/lib.rs Show resolved Hide resolved

tnull force-pushed the 2023-05-expose-builder-in-bindings branch from bb02755 to b2afd61 Compare May 26, 2023 08:31

tnull added 2 commits May 26, 2023 12:42

Break builder pattern and expose setters via bindings

45f5e27

Unfortunately there is no good way of exposing the builder pattern via Uniffi bindings currenlty. We therefore resort to internal mutability and using setters on the `Builder` object.

Expose Network as enum type rather than via string

704e2db

tnull force-pushed the 2023-05-expose-builder-in-bindings branch from b2afd61 to f6c4117 Compare May 26, 2023 10:43

jkczyz reviewed May 26, 2023

View reviewed changes

src/lib.rs Outdated Show resolved Hide resolved

src/lib.rs Outdated Show resolved Hide resolved

tnull force-pushed the 2023-05-expose-builder-in-bindings branch from f6c4117 to 1b4b47a Compare May 26, 2023 20:38

tnull added 3 commits May 26, 2023 22:42

Switch DEFAULT_NETWORK over to Bitcoin mainnet

44dab65

As we're getting ready for release

tnull force-pushed the 2023-05-expose-builder-in-bindings branch from 1b4b47a to de94a54 Compare May 26, 2023 20:42

jkczyz approved these changes May 26, 2023

View reviewed changes

tnull merged commit ab6c009 into lightningdevkit:main May 26, 2023

This was referenced May 27, 2023

Improve logging #108

Merged

Update fee rate cache on startup and make sync interval configurable #102

Merged

tnull mentioned this pull request Jun 13, 2023

Move Builder to own module, allow for shared runtime reference. #115

Merged

joostjager pushed a commit to joostjager/ldk-node that referenced this pull request Jan 28, 2026

Merge pull request lightningdevkit#88 from benthecarman/auth

0a0ebc8

Add HMAC-based authentication for RPC/CLI

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Break builder pattern to expose all setters via bindings #88

Break builder pattern to expose all setters via bindings #88
tnull merged 5 commits intolightningdevkit:mainfrom
tnull:2023-05-expose-builder-in-bindings

tnull commented May 9, 2023 •

edited

Loading

Uh oh!

tnull commented May 12, 2023

Uh oh!

tnull commented May 18, 2023

Uh oh!

tnull commented May 23, 2023

Uh oh!

tnull commented May 25, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tnull commented May 26, 2023 •

edited

Loading

Uh oh!

jkczyz left a comment

Uh oh!

Uh oh!

Uh oh!

tnull commented May 26, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

tnull commented May 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tnull commented May 12, 2023

Uh oh!

tnull commented May 18, 2023

Uh oh!

tnull commented May 23, 2023

Uh oh!

tnull commented May 25, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tnull commented May 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jkczyz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

tnull commented May 26, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

tnull commented May 9, 2023 •

edited

Loading

tnull commented May 26, 2023 •

edited

Loading